Repository navigation
Upstream sync: test isolation, Cursor crash fix, pricing corrections - #36
Merged
Merged
Conversation
Two defects, both measured on the maintainer's machine. Upstream fixed the same two (d6d24a0, 671c1ec); these are ports against this fork. The usage cache defaults to the real Application Support usage-cache.json and had no gate, so any test that ran real providers read and rewrote it. With the app quit, one full suite run changed the real file from 726,929 to 763,881 bytes. Every test run in this fork had been modifying user data. Path injection cannot catch this: it covers the test that injects, not the process. AppEnv.persistsToUserLocation now owns the rule (an injected path always persists, the default path only inside the app) and the cache obeys it. Measured again after the gate: no state file changes across a full run. The companion save was never at risk, since its tests all inject a path. UsageStore defaults its limits providers to the real ones, and about ten tests build a store without a stub. Whenever the Keychain grant happened to be live, a refresh in one of those called api.anthropic.com with the user's real token and spent their rate limit. AppEnv.allowsLiveLimitsFetch now gates the real Claude and Antigravity fetches before any Keychain read, allowing them only in the app or with PTB_PARITY=1. The gate made the two Keychain-discipline tests hollow: blocking before the provider runs makes 'the automatic path never reads the Keychain' true for the wrong reason. Both now open the gate with KeychainReader stubbed, so the provider's real logic runs against a fake Keychain. The manual-path test had been querying the real Keychain with prompts allowed during swift test. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
LocalAdditionalUsageProvider has its own copy of intValue, separate from the LocalUsageReader one that got an upper bound after an overflow crash. This copy had a floor but no ceiling, and NSNumber.intValue saturates at Int.max, so parseCursorBubble's 'input + output' trapped on a corrupt Cursor record. Verified: SIGTRAP with the old helper, clean parse with the clamp. Uses LocalUsageReader.maxParsedTokenValue rather than a second limit. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
chattymin#309) This fork kept the model-family price fallback, so nothing showed as Unavailable, but the fallback was wrong for two models. claude-opus-5-5 was billed at Opus 5's rate (5, 25, 6.25, 0.5) instead of (4, 20, 5, 0.2): 25% high on input, output and cache writes and 2.5x high on cache reads, which dominate real usage. claude-sonnet-5 fell back to Sonnet 4's rate, 1.5x high. Opus 5 happened to match the fallback and is now explicit so it cannot drift. Also adds gemini-2.5-flash-lite. Verified by injection: without the Opus 5.5 row a million cache reads cost 0.5 instead of 0.2. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Seven taken or ported (the Cursor token clamp, two test-isolation fixes, pricing rows, hiding zero-token providers). Two declined on security grounds, both built on the session key declined as chattymin#241. The rest are skipped (tool not installed, or already present), declined (release process, or changing existing behaviour by design), or deferred. Adds a 'deferred' disposition: wanted in principle but colliding with how far the fork has diverged, so it needs a deliberate port rather than a cherry-pick. Forty of the attempted commits conflicted, concentrated in Localization, CompanionStore, PopoverView and CompanionView, which is where porting would risk existing features. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
testStartTransitionsThroughConnectingToWatching failed intermittently on CI
('expected watching, got connecting'), including on main at fac7ea0, a commit
that only bumps a version string. It passed locally.
The stub serves responses from a shared static queue, and each store polls
every 10ms. Only one of the four spectator tests stopped its store, so stores
from earlier tests kept polling and removeFirst() took the next test's queued
response first. That test's own store then got an empty body and never left
connecting. Timing-dependent, hence CI-only.
Every store created by these helpers is now stopped at teardown, and the queue
is reset per test. BattleStoreTests copied the same pattern and stopped none
of its stores; not yet triggered, but the same mechanism, so fixed there too.
25 consecutive runs of both classes: 0 failures.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reviews all 61 upstream commits waiting since the last sync. Seven are taken, each is either security-positive or a correction with no feature impact, and every one of the 61 now has a ledger row.
./scripts/upstream-status.shreports zero unreviewed.Two test-suite defects, measured on the maintainer's machine
The test suite was rewriting the user's real usage cache.
LocalUsageCachedefaults to the realApplication Support/PokeTokenBar/usage-cache.jsonand had no gate, so any test that ran real providers read and rewrote it. With the app quit, one full suite run changed the real file from 726,929 to 763,881 bytes, and a new checksum. Every test run in this fork had been modifying user data.Path injection cannot catch this, because it only covers the test doing the injecting, not the whole test process.
AppEnv.persistsToUserLocationnow owns the rule: an injected path always persists, the default path only inside the app bundle. After the gate, I snapshotted every state file across a full run and none changed. The companion save was never at risk, since its tests all inject a path.Tests could call Anthropic with the user's real token.
UsageStoredefaults its limits providers to the real ones, and about ten tests build a store without a stub. Whenever the Keychain grant happened to be live, a refresh in one of those spent the user's real rate limit, which the running app then backs off from.AppEnv.allowsLiveLimitsFetchnow gates the real Claude and Antigravity fetches before any Keychain read, allowing them only in the app or withPTB_PARITY=1.The gate had a side effect worth calling out: it made both Keychain-discipline tests hollow. Blocking before the provider runs makes "the automatic path never reads the Keychain" true for the wrong reason. Both now open the gate with
KeychainReaderstubbed, so the provider's real logic runs against a fake Keychain. The manual-path test had been querying the real Keychain, prompts allowed, duringswift test.A crash on corrupt Cursor data (chattymin#307)
LocalAdditionalUsageProviderhad its own copy ofintValue, separate from theLocalUsageReaderone that got an upper bound after an earlier overflow crash. This copy had a floor but no ceiling, soparseCursorBubble'sinput + outputtrapped on a corrupt record. Verified: SIGTRAP with the old helper, clean parse with the clamp.Pricing corrections (chattymin#304, chattymin#309, Opus 5.5)
This fork kept the model-family price fallback, so nothing showed as Unavailable, but the fallback was wrong for two models:
claude-opus-5-5(4, 20, 5, 0.2)claude-sonnet-5(2, 10, 2.5, 0.2)Cache reads dominate real usage, so the Opus 5.5 error mattered.
claude-opus-5happened to match the fallback and is now explicit so it cannot drift.Declined on security grounds
chattymin#327 (track several Claude accounts, 3,419 lines) and the per-account session key are both built on the session key declined as chattymin#241, which stores a full account cookie in plaintext.
Deferred
Forty of the attempted commits conflicted, concentrated in
Localization,CompanionStore,PopoverViewandCompanionView: the files this fork has diverged most. Porting features into them is where existing behaviour breaks, so they are recorded as a newdeferreddisposition (wanted in principle, needs a deliberate port) rather than forced in. The most valuable of these is protecting legendary companions from accidental egg discard, which prevents data loss and is worth its own change.Upstream's chattymin#334 independently arrived at exactly the Rare Candy re-arm design from #29, which is recorded as already present.
Verification
1072 tests, 0 failures, 90.27% coverage. All 24 hardening checks pass. Each fix verified by injection.
🤖 Generated with Claude Code